Doxygen comments fixes #5372

SenRamakri · 2017-10-24T15:47:51Z

This is the change for updating/fixing Doxygen comments/config to better organize the doxygen docs.

sg- · 2017-10-25T17:54:48Z

drivers/UARTSerial.h

@@ -39,6 +39,13 @@

 namespace mbed {

+/** \addtogroup drivers */
+
+/** A serial port implementation


This could use some work. It sounds so boring.

0xc0170 · 2017-10-25T17:54:10Z

rtos/rtos_idle.h

+ */
+/** 
+ @note
+ Sets the hook function called by by idle task.


by just once

0xc0170 · 2017-10-25T17:54:50Z

rtos/rtos_idle.h

+/** 
+ @note
+ Sets the hook function called by by idle task.
+ @param fptr Hook function pointer.


. should not be there?

AnotherButler · 2017-10-25T18:10:49Z

platform/mbed_assert.h

@@ -39,6 +43,19 @@ void mbed_assert_internal(const char *expr, const char *file, int line);
 }
 #endif

+/** MBED_ASSERT
+ *  Declare runtime assertions, results in runtime error if condition is false


This is a nitpick, but please change the comma to a semicolon

sg-

I dont understand all the changes to groups and why they're needed. the commit history needs to be rewritten or all part of one commit.

sg- · 2017-10-25T23:28:07Z

doxyfile_options

@@ -32,7 +32,7 @@ DOXYFILE_ENCODING      = UTF-8
 # title of most generated pages and in a few other places.
 # The default value is: My Project.

-PROJECT_NAME           = "My Project"
+PROJECT_NAME           = "Mbed OS Reference"


This change is also needed in the json version

Can you please point me to which json file I should add this change? Then the group changes are only to make navigation through the API docs easier. It doesn't change or move any APIs or the groups. See the attached image. It shows how the grouping/API looks like with these changes. Earlier everything was showing up in the index.html page itself and this change cleans up lot of those. Its based on our discussion to align things like how HAL group was organized.

doxyfile_options.json

@sg- The contents of json file is as below -
{
"ENABLE_PREPROCESSING": "YES",
"MACRO_EXPANSION": "YES",
"EXPAND_ONLY_PREDEF": "NO",
"SEARCH_INCLUDES": "YES",
"INCLUDE_PATH": "",
"INCLUDE_FILE_PATTERNS": "",
"PREDEFINED": "DOXYGEN_ONLY DEVICE_ANALOGIN DEVICE_ANALOGOUT DEVICE_CAN DEVICE_ETHERNET DEVICE_EMAC DEVICE_FLASH DEVICE_I2C DEVICE_I2CSLAVE DEVICE_I2C_ASYNCH DEVICE_INTERRUPTIN DEVICE_LOWPOWERTIMER DEVICE_PORTIN DEVICE_PORTINOUT DEVICE_PORTOUT DEVICE_PWMOUT DEVICE_RTC DEVICE_TRNG DEVICE_SERIAL DEVICE_SERIAL_ASYNCH DEVICE_SERIAL_FC DEVICE_SLEEP DEVICE_SPI DEVICE_SPI_ASYNCH DEVICE_SPISLAVE DEVICE_STORAGE "MBED_DEPRECATED_SINCE(f, g)=" "MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, M)="",
"EXPAND_AS_DEFINED": "",
"SKIP_FUNCTION_MACROS": "NO",
"EXCLUDE_PATTERNS": "/tools/ /targets/ /FEATURE_/* /features/mbedtls/ /features/storage/ /features/unsupported/ /features/filesystem/ /BUILD/ /rtos/TARGET_CORTEX/rtx/* /cmsis/ /features/FEATURES_"
}
Looks like not changes are needed in that, can you please confirm?

If you dont add that here, it will not be present in the online hosted version. All changes to the local doxyfile need to be accounted for in the json version. Otherwise it is assumed that the tool defaults will be used.

Ah yes, I see your point now, updated json as well now. Please review.

0xc0170 · 2017-10-26T15:01:08Z

👍 for adding the groups. You could have added a screenshot how it looked prior this change and with this change. To get a picture of how this patch organizes the doc for our API.

As pinpointed above, can you squash these changes into one (I believe all of them can be done within one commit - same logical change for all the files).

I noticed also some are called functions and some class but those functions like CThunk, it is actually a class, can you also check those?

SenRamakri · 2017-10-26T20:39:35Z

Review comments fixed with squashed commit.

0xc0170

One question regarding debug builds, if that note is correct. The rest LGTM

0xc0170 · 2017-10-27T16:26:36Z

platform/mbed_assert.h

+ *  Declare runtime assertions: results in runtime error if condition is false
+ *
+ *  @note
+ *  Use of MBED_ASSERT is limited debug builds only.


Same question here as in the docs, debug builds are ?

SenRamakri · 2017-10-31T13:28:28Z

@sg- I have fixed all the review comments. can you please look into this when you get a chance?

SenRamakri · 2017-11-02T16:02:28Z

@sg- Can you please review if there is anything pending on this PR from my side?

0xc0170 · 2017-11-03T15:25:56Z

/morph build

mbed-ci · 2017-11-03T16:43:39Z

Build : SUCCESS

Build number : 426
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5372/

Triggering tests

/morph test
/morph uvisor-test

mbed-ci · 2017-11-03T21:48:54Z

Test : FAILURE

Build number : 217
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5372/217

studavekar · 2017-11-06T20:13:19Z

/morph test

mbed-ci · 2017-11-07T00:08:04Z

Test : SUCCESS

Build number : 258
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5372/258

SenRamakri requested review from sg- and AnotherButler October 24, 2017 19:56

sg- reviewed Oct 25, 2017

View reviewed changes

0xc0170 reviewed Oct 25, 2017

View reviewed changes

0xc0170 added the needs: review label Oct 25, 2017

AnotherButler reviewed Oct 25, 2017

View reviewed changes

AnotherButler approved these changes Oct 25, 2017

View reviewed changes

sg- suggested changes Oct 25, 2017

View reviewed changes

Doxygen comment updates and fixes

3ad2984

SenRamakri force-pushed the sen_PlatformDoxygenUpdates branch from f737a1a to 3ad2984 Compare October 26, 2017 20:37

0xc0170 approved these changes Oct 27, 2017

View reviewed changes

Fix comments on mbed_assert.h

37214ab

SenRamakri mentioned this pull request Oct 30, 2017

Docs for MemoryStats API, Debug API, Assert and Error APIs ARMmbed/mbed-os-5-docs#310

Merged

Fixed doxygen_options.json

1ba418e

sg- approved these changes Nov 2, 2017

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Nov 3, 2017

0xc0170 added ready for merge release-version: 5.6.5 and removed needs: CI labels Nov 7, 2017

0xc0170 merged commit fbd9e7e into ARMmbed:master Nov 9, 2017

0xc0170 removed the ready for merge label Nov 9, 2017

Doxygen comments fixes #5372

Doxygen comments fixes #5372

Uh oh!

Conversation

SenRamakri commented Oct 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sg- left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Oct 26, 2017

Uh oh!

SenRamakri commented Oct 26, 2017

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SenRamakri commented Oct 31, 2017

Uh oh!

SenRamakri commented Nov 2, 2017

Uh oh!

0xc0170 commented Nov 3, 2017

Uh oh!

mbed-ci commented Nov 3, 2017

Triggering tests

Uh oh!

mbed-ci commented Nov 3, 2017

Uh oh!

studavekar commented Nov 6, 2017

Uh oh!

mbed-ci commented Nov 7, 2017

Uh oh!

Uh oh!